Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(react): BREAKING CHANGE Upgrade to React 18 #3556

Merged
merged 26 commits into from
Jul 12, 2024

Conversation

greg-in-a-box
Copy link
Contributor

@greg-in-a-box greg-in-a-box commented May 21, 2024

BREAKING CHANGE

These are the following changes:

  • React upgraded to 18
    • Updated types
    • act comes from react now instead of `react-dom/test-utils
    • renderHook comes from @testing-library/react instead @testing-library/react-hooks
    • Updated testing libraries
    • Updated peer dependency to support 17 and 18
  • Storybook upgraded to 8

Checklist

  • Validate Storybook
  • Validate styleguidist
  • Validate linters
  • Validate builder

@greg-in-a-box greg-in-a-box force-pushed the react-18 branch 2 times, most recently from 78c1408 to 7d5d2e9 Compare July 1, 2024 21:57
@greg-in-a-box greg-in-a-box changed the title WIP feat(react): BREAKING CHANGE Upgrade to React 18 Jul 2, 2024
@greg-in-a-box greg-in-a-box force-pushed the react-18 branch 2 times, most recently from ab7c72d to 926275a Compare July 8, 2024 17:24
@@ -289,6 +289,7 @@ describe('elements/content-sharing/SharingModal', () => {
expect(wrapper.exists(LoadingIndicator)).toBe(true);

wrapper.update();
console.log(wrapper.debug());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove console.log()

@greg-in-a-box greg-in-a-box linked an issue Jul 9, 2024 that may be closed by this pull request
Comment on lines +9 to +11
Object.defineProperty(global, 'TextEncoder', {
value: util.TextEncoder,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error was thrown and said to add this to the setup

Comment on lines 2 to 4
import { boolean } from '@storybook/addon-knobs';

import { State, Store } from '@sambego/storybook-state';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both deprecated, removed from all stories

@@ -23,28 +23,9 @@ exports[`components/collapsible/Collapsible should apply buttonProps correctly 1
/>
</PlainButton>
</div>
<AnimateHeight
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated after bumping react-animate-height to 3

import * as React from 'react';
import { IntlProvider } from 'react-intl';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IntlProvider is handle via a storybook addon now, removed from all stories

@@ -1,8 +1,7 @@
// @flow
/* eslint-disable react-hooks/rules-of-hooks */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added this to avoid having to capitalize all stories to and keeping this as camelcase

Comment on lines +34 to +35
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Comment on lines +306 to +311
act(() => {
wrapper.setState({ approvers: [approver] });
});
act(() => {
wrapper.instance().handleApproverSelectorSelect([newApprover]);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they need to be separated

@greg-in-a-box greg-in-a-box marked this pull request as ready for review July 10, 2024 23:54
@greg-in-a-box greg-in-a-box requested review from a team as code owners July 10, 2024 23:54
@@ -1,12 +1,13 @@
import React from "react";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eslint?

package.json Outdated
"*.js": ["eslint --fix", "git add"],
"*.ts": ["eslint --ext=.ts --fix", "git add"],
"*.tsx": ["eslint --ext=.tsx --fix", "git add"],
"*.js|ts|tsx": ["eslint --fix", "git add"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did this syntax come from? feel like it's not doing what we think it should

im seeing something like *.{js,ts,tsx} on the docs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didint know that was being used for something, thought it wa s just a label, updated in separate PR for eslint

@greg-in-a-box greg-in-a-box force-pushed the react-18 branch 2 times, most recently from c9ed99d to a7e00bb Compare July 12, 2024 01:44
@@ -5,6 +5,8 @@
*/

import * as React from 'react';
// TODO switch to createRoot when upgrading to React 18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need a ticket to track this one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will break on its own when we head to 19

tjiang-box
tjiang-box previously approved these changes Jul 12, 2024

<Meta of={ContentSharingStories} />
<Meta />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's this doing now if stories is removed?

@greg-in-a-box greg-in-a-box merged commit 91e0978 into box:master Jul 12, 2024
7 checks passed
kajarosz pushed a commit to kajarosz/box-ui-elements that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for React 18
3 participants